perf(build): parallelize fragmented backing reads#2872
Open
ValentaTomas wants to merge 6 commits into
Open
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit a607c1a. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Parallel path skips planned reads
- Modified planRead to return accumulated segments with io.EOF when progress has been made, readAtParallel to execute segments even with EOF, and ReadAt to properly propagate partial read counts.
Or push these changes by commenting:
@cursor push edbb24779f
Preview (edbb24779f)
diff --git a/packages/orchestrator/pkg/sandbox/build/build.go b/packages/orchestrator/pkg/sandbox/build/build.go
--- a/packages/orchestrator/pkg/sandbox/build/build.go
+++ b/packages/orchestrator/pkg/sandbox/build/build.go
@@ -64,6 +64,8 @@
if maxParallel > 1 && len(p) > 0 {
if err := b.readAtParallel(ctx, p, off, maxParallel); err == nil {
return len(p), nil
+ } else if err == io.EOF {
+ return len(p), io.EOF
} else if shouldRetrySerial(err) {
if retry, swapErr := b.retryOnTransition(ctx, err); !retry && swapErr != nil {
return 0, swapErr
@@ -198,10 +200,13 @@
}
func (b *File) readAtParallel(ctx context.Context, p []byte, off int64, maxParallel int) error {
- segments, err := b.planRead(ctx, p, off)
- if err != nil {
- return err
+ segments, planErr := b.planRead(ctx, p, off)
+ if planErr != nil && planErr != io.EOF {
+ return planErr
}
+ if len(segments) == 0 {
+ return planErr
+ }
if len(segments) <= 1 {
for _, s := range segments {
n, err := s.diff.ReadAt(ctx, p[s.dstOff:s.dstOff+int(s.length)], s.srcOff, s.ft)
@@ -213,7 +218,7 @@
}
}
- return nil
+ return planErr
}
g, gctx := errgroup.WithContext(ctx)
@@ -233,7 +238,11 @@
})
}
- return g.Wait()
+ if err := g.Wait(); err != nil {
+ return err
+ }
+
+ return planErr
}
func (b *File) planRead(ctx context.Context, p []byte, off int64) ([]readSegment, error) {
@@ -255,6 +264,9 @@
}
readLength := min(int64(mappedToBuild.Length), int64(len(p)-n))
if readLength <= 0 {
+ if n > 0 {
+ return segments, io.EOF
+ }
return nil, io.EOF
}
if mappedToBuild.BuildId == uuid.Nil {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit dbbe148. Configure here.
❌ 4 Tests Failed:
View the full list of 4 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Calling CompressionType() directly on the result of h.GetBuildFrameData(buildID) will cause a nil pointer dereference panic if the build is uncompressed, as GetBuildFrameData returns nil in that case. To prevent this, check if the returned frame table is non-nil before retrieving its compression type.
dobrac
approved these changes
May 30, 2026
Contributor
dobrac
left a comment
dobrac
left a comment
There was a problem hiding this comment.
The implementation complexity is quite high - not sure if needs to be.
One nit otherwise with nil handling
Remove defensive nil handling around build read parallelism since build files are created with a DiffStore.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parallelize fragmented build reads when a request spans multiple backing mappings, instead of reading them serially. Gated by
max-parallel-build-read-segments(default 1 = serial). Applies to both memfile and rootfs viabuild.File.ReadAt, and to multi-mappingSlice(which falls back throughReadAt).Serial and parallel paths share one mapping planner and segment reader; cache eviction or a peer transition re-resolves and retries.